Skip to content

Conversation

@kumarUjjawal
Copy link
Contributor

Which issue does this PR close?

Rationale for this change

The helper functions compute_truncate32 and compute_truncate64 incorrectly use .round() instead of .trunc(), causing incorrect results:

  • trunc(3.76, 1) returned 3.8 (wrong - rounded)
  • trunc(3.76, 1) should return 3.7 (correct - truncated)

What changes are included in this PR?

  • Change .round() to .trunc() in compute_truncate32 and compute_truncate64
  • Update unit test expected values to reflect correct truncation behavior

Are these changes tested?

Yes. Unit tests updated and pass.

Are there any user-facing changes?

@github-actions github-actions bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Jan 13, 2026
Copy link
Contributor

@Jefffrey Jefffrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch


assert_eq!(floats.len(), 5);
assert_eq!(floats.value(0), 15.0);
assert_eq!(floats.value(1), 1_234.268);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this assertion removed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I might have accidently deleted the line during editing.

@Jefffrey Jefffrey added this pull request to the merge queue Jan 16, 2026
Merged via the queue into apache:main with commit 5d68b75 Jan 16, 2026
28 checks passed
@Jefffrey
Copy link
Contributor

Thanks @kumarUjjawal & @martin-g

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trunc function with precision uses round instead of trunc semantics

3 participants